Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement view resizing on keyboard show/hide #1119

Merged
merged 4 commits into from
Jan 23, 2019
Merged

Implement view resizing on keyboard show/hide #1119

merged 4 commits into from
Jan 23, 2019

Conversation

MarkChrisLevy
Copy link
Contributor

@MarkChrisLevy MarkChrisLevy commented Jan 11, 2019 •
edited
Loading

Very simple implementation of view resizing when keyboard showing/hidding. The logic is copied from cordova-plugin-ionic-keyboard. There are two config parameters related to resizing:

  • resize, allowed value is either true or false. Default value is true.
  • resizeMode, allowed value is native, ionic or body. Default value is native.

@MarkChrisLevy
Copy link
Contributor Author

Imho there could be only one config param resize, with values native, ionic, body or none. Agree?

@sneko
Copy link

sneko commented Jan 13, 2019 •
edited
Loading

I can confirm it solves my issue described there: ionic-team/ionic-framework#16797 (cc @paulstelzer ).

I noticed a little issue with keyboard showing up slower than the content area being resized. So it renders a white space during the transition. You can see it in the video attached below:
ScreenRecording_01-13-2019 12-44-22.zip

Also, I'm still facing the known offset issue described here: #814 . There is no specific steps to reproduce, it's random :/

Thanks for the PR :)

@sneko
Copy link

sneko commented Jan 13, 2019 •
edited
Loading

While inspecting the ion-content element with my browser and my iPhone, I can see this white space is not part of the ion-content nor the ion-app.

I can confirm it by setting a background color to it and it doesn't change this space color.

But I figured out it works when setting the background color on either the <html or the <body element.

@sneko sneko mentioned this pull request Jan 13, 2019
Closed
@mlynch
Copy link
Contributor

mlynch commented Jan 14, 2019

Thanks for the PR. resize is a bit too generic, maybe keyboardResize? cc @jcesarmobile

@jcesarmobile
Copy link
Member

@mlynch getConfig picks the config for the current plugin, so resize will be inside a Keyboard object

"plugins": {
    "Keyboard": {
        "resize": "none"
    }
}

@MarkChrisLevy
Copy link
Contributor Author

MarkChrisLevy commented Jan 15, 2019 •
edited
Loading

I made a change in config parameters, only one parameter needed "resize" with possible values native, ionic, body or none. By default resizing is enabled and set to native mode. So in order to set keyboard resizing parameter, you need to define it in capacitor.config.json:

"plugins": {
    "Keyboard": {
        "resize": "none"
    }
}

"plugins": {
    "Keyboard": {
        "resize": "ionic"
    }
}

stripathix
stripathix reviewed Jan 22, 2019
View reviewed changes
ios/Capacitor/Capacitor/Plugins/Keyboard.m Outdated
}
}
// NSLog(@"CAPIonicKeyboard: resize mode %d", self.keyboardResizes);

if (self.keyboardResize == ResizeNative) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think It should be keyboardResizes instead of keyboardResize. It throws an error that keyboardResize is not defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. The "s" was lost, when I changed config param name. Thanks!

jcesarmobile
jcesarmobile approved these changes Jan 23, 2019
View reviewed changes
Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.
I've tested it and works fine.

Merging, thanks!

@jcesarmobile jcesarmobile merged commit df5d93c into ionic-team:master Jan 23, 2019
@jcesarmobile jcesarmobile mentioned this pull request Jan 23, 2019
Closed
@sneko sneko mentioned this pull request Jan 23, 2019
Closed
@sneko
Copy link

sneko commented Jan 26, 2019 •
edited
Loading

Any idea to fix the difference of synchronization between the keyboard showing up and the content area as I posted in a reply of this PR (#1119 (comment))? @EricKit also described this behavior: ionic-team/ionic-framework#16797 (comment)

On another side, if I focus on an input, the keyboard will show up, but if now I try to scroll the view, the view have its padding scrollable here is the result:
image

Note: using released Ionic 4.0

@EricKit EricKit mentioned this pull request Jan 26, 2019
Closed
@jcesarmobile
Copy link
Member

Please, don’t comment on closed issues or merged PRs. If something is not working fine report a new issue, it’s very hard to keep track of problems if they are “hidden”. I saw it this time, but might not see it next time, and even if I saw it I might forget

This was referenced Jan 27, 2019
Closed
Closed
@MarkChrisLevy MarkChrisLevy deleted the keyboard-resize branch February 6, 2019 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stripathix stripathix stripathix left review comments

@jcesarmobile jcesarmobile jcesarmobile approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@MarkChrisLevy @sneko @mlynch @jcesarmobile @stripathix